Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppStats: Connection Duration Addition to API #908

Merged
merged 10 commits into from
Oct 12, 2021

Conversation

alexadhy
Copy link
Contributor

@alexadhy alexadhy commented Oct 4, 2021

Did you run make format && make check? yes

Fixes #895

Changes:

  • Added Connection Duration to the vpn-client

TODO:

  • Adds the duration to the API when user requires visor overview / connections stats.
  • Also generates the mock and readmegen

How to test this PR:

  • Run the vpn-client app from the visor via UI / cli
  • Run the VPN UI
  • Connect to any vpn-server
  • Check the connection duration field in json

Screen Shot 2021-10-04 at 21 43 02

@alexadhy alexadhy marked this pull request as ready for review October 5, 2021 03:36
@alexadhy alexadhy changed the title [WIP] AppStats: Connection Duration Addition to API AppStats: Connection Duration Addition to API Oct 5, 2021
@alexadhy
Copy link
Contributor Author

alexadhy commented Oct 5, 2021

@Senyoret1 can you also try this?

Copy link
Contributor

@mrpalide mrpalide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. Functionality is OK and code is clear to me.

@Senyoret1
Copy link
Contributor

I did not check the code, but the functionality appears to be working well. Is the time in seconds?

@alexadhy
Copy link
Contributor Author

alexadhy commented Oct 7, 2021

I did not check the code, but the functionality appears to be working well. Is the time in seconds?

it is, is it ok ?

@Senyoret1
Copy link
Contributor

Yes, seconds is ok for me

@ersonp
Copy link
Contributor

ersonp commented Oct 7, 2021

Everything looks good. I just had one suggestion. We should send connection_duration here http://localhost:8000/api/visors/<pk>/apps/vpn-client/connections

[
   {
      "is_alive":true,
      "latency":57,
      "upload_speed":2283,
      "download_speed":411,
      "bandwidth_sent":215672,
      "bandwidth_received":235619,
      "error":""
   }
]

@alexadhy
Copy link
Contributor Author

alexadhy commented Oct 7, 2021

@ersonp done

Copy link
Contributor

@ersonp ersonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good and everything is working as intended.

@jdknives jdknives merged commit 244adc2 into skycoin:develop Oct 12, 2021
@alexadhy alexadhy deleted the vpn-conn-duration branch November 23, 2021 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants